Skip to content

Merging in Emily Ball's Isca-Mars developments - #302

Open
sit23 wants to merge 50 commits into
ExeClim:masterfrom
sit23:mars_dust_lh
Open

Merging in Emily Ball's Isca-Mars developments#302
sit23 wants to merge 50 commits into
ExeClim:masterfrom
sit23:mars_dust_lh

Conversation

@sit23

@sit23 sit23 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

As discussed in #299, it has been my intention for a while to merge Emily Ball's Isca-Mars version into the main Isca. This version was forked from Isca in 2019, and built off my earlier Mars configurations, but now including the option to have latent heating from CO2 condensation, and also a representation of Martian dust using Socrates and prescribed dust fields. I have used Claude to cherry-pick commits from Emily's version to implement this into the main version of Isca, and I have begun testing this version to make sure it doesn't break existing functionality and so that the Mars dust test case works as expected.

emilyrball and others added 30 commits August 13, 2026 12:22
(cherry picked from commit 4ce1280)
(cherry picked from commit 46f2ef6)
Changed module/routine names etc to lscale_cond_lh

(cherry picked from commit 9d20364)
(cherry picked from commit 958daa1)
Include atmos_param/lscale_cond_lh/lscale_cond_lh.F90

(cherry picked from commit b98deef)
Include atmos_param/lscale_cond_lh/lscale_cond_lh.F90

(cherry picked from commit 1be3277)
Include atmos_param/lscale_cond_lh/lscale_cond_lh.F90

(cherry picked from commit 2ec233a)
(cherry picked from commit 8c61ed5)
Include atmos_param/lscale_cond_lh/lscale_cond_lh.F90

(cherry picked from commit 4ca9008)
Temperature change to account for CO2 condensation

(cherry picked from commit 11beb1c)
Added dt_tg as input in lscale_cond_lh

(cherry picked from commit bfd934c)
(cherry picked from commit 7f2c985)
(cherry picked from commit 403b943)
(cherry picked from commit eb3ab68)
(cherry picked from commit 22fc5a9)
(cherry picked from commit 32499cc)
(cherry picked from commit ff1ef3a)
(cherry picked from commit 28cb4dc)
(cherry picked from commit ace0b26)
(cherry picked from commit 5792960)
(cherry picked from commit f3dbf06)
(cherry picked from commit 6f7bfb8)
(cherry picked from commit 1363cf5)
emilyrball and others added 18 commits August 13, 2026 12:46
(cherry picked from commit 4e2a448)
The dust profile computation in run_socrates was gated on
some_dust_condition, a flag that defaulted to .TRUE. and was never
added to the namelist, so it could not be turned off and silently fed
a nonzero dust aerosol mixing ratio (via dust_mix_ratio's default of
1.0e-06) into every Socrates radiation call, regardless of whether
Mars dust forcing was actually wanted. Renamed to do_dust_forcing,
defaulted to .FALSE., and added to socrates_rad_nml, matching the
gating pattern used for the other new physics options on this branch.
…test case

Moves the ported socrates_mars_dust_lh.py into
exp/test_cases/socrates_mars_dust/socrates_mars_dust_test_case.py, matching the
current test-case conventions (module-level Experiment/diag_table/namelist,
compile in __main__, no per-loop Experiment construction since all the swept
lists were single-element). Adds do_dust_forcing:True, now required after the
gating fix. Drops the superseded intermediate socrates_mars_lh.py (LH-only, no
dust) in favour of this combined dust+LH version. Documents the required
externally-sourced spectral/dust-climatology files in input/README.md,
following the same pattern as the existing socrates_mars test case.
Mirrors the existing socrates_mars entry: an import block in
get_experiment_details_from_test_case_name(), and a commented-out entry in
the default test-case list pointing at input/README.md, since it also needs
externally-sourced files before it can run.
Follow-up to the previous commit, which removed the old files but missed
staging the new ones.
Missed during conflict resolution when merging in the do_lscale_cond_lh
dry-convection consistency check - gfortran rejects comparing two
logicals with .eq., it must be .eqv. Caught by a full compile.
Matches the existing explicit lscale_cond_nml block for consistency, since
this test case actually uses do_lscale_cond_lh rather than do_lscale_cond.
Replaces the standalone lscale_cond_lh_mod with a new subroutine,
lscale_cond_co2_mars, living alongside lscale_cond in the same module -
one file, one namelist, rather than a near-duplicate copy of the whole
module for a single alternative subroutine. Renamed for clarity: this is
Mars/CO2-specific physics (temperature-threshold condensation against a
pressure-dependent frost point), not a general Earth-applicable option
like the water-vapor lscale_cond it sits beside.

The four CO2 thermodynamic constants (L_c0, c_pg, c_pc, c_p) are scoped
locally to the new subroutine rather than at module level, with comments
explaining what each represents - including a note that c_pg/c_p (~736-770
J/kg/K) don't match CP_AIR (~845 J/kg/K for the Mars test cases' configured
rdgas/kappa, used everywhere else in the model's energy budget:
two_stream_gray_rad, socrates_interface, spectral_dynamics, mixed_layer,
hs_forcing), and should be checked for consistency before the implied
condensed mass is trusted quantitatively.

Also drops the do_evap/precip_evap call this subroutine inherited from
lscale_cond_lh: it depends on qsat/dqsat/hlcp, none of which this
temperature-threshold formulation ever computes, so it was reading
uninitialized memory regardless of the do_evap namelist setting - as was
the qdel/precip output more generally, now explicitly zeroed rather than
left as whatever was previously on the stack. tdel/lh_rel (the only
physics currently live in this scheme) are unchanged.

lscale_cond_init()/lscale_cond_end() (already called unconditionally by
idealized_moist_phys) now serve both subroutines - no separate init/end
or namelist needed for the Mars variant.
… mismatch

Same bug as socrates_mars_test_case.py (which this test case's namelist was
originally copied from, on the socrates_mars_tcmax_fix branch off master):
sat_vapor_pres.F90 declares tcmin/tcmax as INTEGER, but tcmax was set with a
trailing '.' (350.), making it a Python float that gets serialized as
350.0 - a namelist type mismatch gfortran correctly rejects, surfaced by
FMS's check_nml_error as a FATAL naming sat_vapor_pres_nml.

Confirmed by bisection (removing/varying individual namelist assignments)
and by directly running this test case to completion after the fix.

Once socrates_mars_tcmax_fix lands on master and this branch is synced
with it, this commit's diff should already match - nothing further to do.
Found via trip_test: plain socrates_aquaplanet (no dust, standard ga7
spectral file, n_aerosol=0) crashed with "lookup_es_1d: saturation vapor
pressure table overflow" on this branch but not on master, in an isolated,
cache-cleared, reproducible A/B comparison - a real behavioural difference
between the upstream set_aer_mod (which only allocates the aerosol
structure and never touches mr_source when there's nothing to set) and
this branch's replacement socrates_set_aer, which unconditionally set
aer%mr_source even for spectral files with zero aerosol species.

Wrapping the mr_source assignment and mix_ratio loop in
`n_aerosol > 0` restores exact equivalence to the original set_aer_mod
whenever a spectral file (like the standard, non-Mars ga7 files) defines
no aerosols, while leaving the dust-forcing behaviour for Mars-dust
spectral files (which do define aerosol species) unchanged.
The real fix for the socrates_aquaplanet crash the previous commit
(gating socrates_set_aer's mr_source assignment) didn't fully resolve -
read_control.F90 unconditionally flipped control%l_aerosol from .FALSE.
to .TRUE. for every Socrates run, Mars/dust or not. This is the top-level
switch that tells Socrates' core radiative transfer to apply aerosol
effects at all, so forcing it on unconditionally activated aerosol
processing even for spectral files that define zero aerosol species
(the standard, non-Mars ga7 files) - destabilizing an ordinary aquaplanet
run enough to blow the saturation-vapor-pressure lookup table within the
first few days of a cold start.

Same gating condition as the previous commit: only switch it on when
spectrum%aerosol%n_aerosol > 0.
…osol

The standard ga7 spectral files used by every non-Mars Socrates run also
carry aerosol species blocks (inherited from the Met Office UM) even
though Isca never wires up their parametrizations, so
spectrum%aerosol%n_aerosol > 0 is true for those too. Gating l_aerosol on
that check switched aerosol radiative effects on for ordinary runs,
crashing Socrates' opt_prop_aerosol with "i_aerosol_parametrization ...
illegal value". do_dust_forcing (the namelist flag for the Mars dust
scheme) is the correct switch: it reflects what the user actually asked
for, independent of what a given spectral file happens to contain.
main_nml's days/seconds units, and DiagTable's 'days' time_units, always
mean Earth's fixed 86400s/day regardless of planet. The Mars test cases
(grey_mars, radiative_eq_mars, socrates_mars, socrates_mars_dust) instead
use a calendar Mars day of 88440s (dt_atmos=110s, 804 steps/day) so that
an integer number of days fits into a Mars year - and 86400 is not a
multiple of 110, so the generic 3-Earth-day short-run override crashes
with "run length must be multiple of atmosphere time step" for these
cases. Override to 3*88440s instead, with a matching Mars-specific diag
table using the same day length for its output cadence.
Same fix as radiative_eq_mars_nml_fix (for master): do_newtonian_cooling_as_rad
and newt_relax_surface aren't declared Fortran namelist variables anywhere in
the source, so idealized_moist_phys_nml fails FMS's mistyped-namelist check.
Applied here directly too since trip_test's driver script always reads test
case definitions from the live GFDL_BASE checkout regardless of which commit
is being compiled, so this branch needs its own copy of the fix to run
radiative_eq_mars at all.
Same fix as radiative_eq_mars_nml_fix: update_sst_from_fluxes is not a
declared Fortran namelist variable in mixed_layer_nml. Found by actually
running the test case, which failed on this key next after the previous
commit's fix.
Neither exists anywhere as a declared Fortran namelist variable (confirmed
via full-source grep) - same class of bug as the previous two commits.
With these gone, radiative_eq_mars now runs cleanly and matches master
bit-for-bit, confirming no Mars-dust-physics regression leaked into this
unrelated test case.
These keys (do_newtonian_cooling_as_rad, newt_relax_surface,
update_sst_from_fluxes, rh_target, delta_t_relax) aren't declared on
current master's Fortran namelists, but they likely originate from - and
work correctly on - the much older top_down_with_moisture branch, which
predates the code they depend on being removed/renamed. Rather than
deleting them here, the plan is to reconcile a modernized version of
top_down_with_moisture instead, so this test case is expected to fail
until that lands.
@sit23

sit23 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Updating on the trip test results (Claude's summary).

Trip test results so far

Comparing this branch (327dd8c1) against master (d1321ac3).

Test case Result
socrates_aquaplanet ✅ bit-identical
socrates_aquaplanet_cloud ✅ bit-identical
ape_aquaplanet ✅ bit-identical
grey_mars ✅ bit-identical
socrates_mars ✅ bit-identical
socrates_mars_dust ✅ runs cleanly (new capability, no master equivalent to diff against)
radiative_eq_mars ❌ fails — known issue, see below, not caused by this branch

Two real fixes found along the way

Socrates aerosol effects were on for every run, not just Mars dust. The
dust-forcing port had flipped control%l_aerosol unconditionally to .TRUE.
in read_control.F90 (master hardcodes it .FALSE.), which broke every
plain Socrates run — crashing with a saturation-vapor-pressure overflow, a
downstream symptom of Socrates' aerosol code choking on species the standard
ga7 spectral file defines but never configures a parametrization for.
Fixed by gating on do_dust_forcing (the namelist flag for the actual
feature) instead of switching it on for every run.

Mars trip tests crashed on run length. FMS's main_nml days/seconds
units always assume Earth's fixed 86400s/day. The Mars test cases use a
calendar day of 88440s (dt_atmos=110s) so an integer number of days fits a
Mars year — 86400 isn't a multiple of 110, so the trip test framework's
generic short-run override crashed on every Mars case. Added a Mars-specific
override (3×88440s) to the trip test driver only; no changes to master.

Known failing case: radiative_eq_mars (not a regression from this branch)

Fails identically on both master and this branch with an "unknown
namelist variable" error — five keys in
radiative_eq_mars_test_case.py (do_newtonian_cooling_as_rad,
newt_relax_surface, update_sst_from_fluxes, rh_target, delta_t_relax)
aren't declared anywhere in current master's Fortran.

These almost certainly belong to top_down_with_moisture (last touched
2020-04-24, ~979 commits behind master) — the branch this test case's physics
originates from — and were lost when this test case was merged without that
branch ever being reconciled. Deleting the keys locally would make the test
pass but paper over the real gap, so that's deliberately not done here.

Plan: bring a modernized top_down_with_moisture up to date against
master on its own branch/PR, then use that to properly restore
radiative_eq_mars. Until then this failure is expected and acknowledged,
not something this PR is trying to fix.

sit23 added 2 commits August 18, 2026 18:05
Comment-only change, no behaviour change. These are two separate gates on
the Mars dust profile applied at different stages (computation vs.
whether it reaches the radiative transfer call), easy to mistake for
redundant since they're both required for the normal dust-on case. Added
matching explanations at both declarations (socrates_config_mod.f90),
both usage sites (socrates_interface.F90), the read_control.F90 aerosol
gate that deliberately uses only one of the two, and the dust test case's
namelist where both are set.
Matches do_dust_forcing's default and the "everything new defaults off"
pattern used throughout this branch: previously, setting do_dust_forcing
alone (without touching account_for_effect_of_dust) was enough to
silently get dust radiative heating, since the latter defaulted .TRUE.
like its water/ozone counterparts. Now both flags must be explicitly set
to get the normal dust-on case, as socrates_mars_dust_test_case.py already
does. No effect on any currently-passing test case - it's the only place
do_dust_forcing is used, and it already sets account_for_effect_of_dust
explicitly.
@sit23

sit23 commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Have now run a full trip test suite with these changes. Results (summarised by Claude) are below:

Full trip test suite results

Comparing mars_dust_lh (63e2f3b2) against master (d1321ac3) — all 19
test cases in trip_test_command_line's default suite, plus socrates_mars
and socrates_mars_dust (excluded from the default suite only because they
need external data files, per their input/README.md).

Summary

Result Count Test cases
✅ Bit-identical to master 18 bucket_model, frierson, held_suarez, MiMA, realistic_continents_fixed_sst, realistic_continents_variable_qflux, socrates_aquaplanet, socrates_aquaplanet_cloud, top_down_test, variable_co2_grey, variable_co2_rrtm, ape_aquaplanet, barotropic_vort_eq_stirring, shallow_water_stirring, column_test, grey_mars, socrates_mars
✅ Runs cleanly (new capability, no master equivalent) 1 socrates_mars_dust
❌ Pre-existing failure, unrelated to this branch 2 axisymmetric, giant_planet
❌ Known, acknowledged failure 1 radiative_eq_mars

Every test case this branch could plausibly affect is bit-identical to
master.
The only failures are either pre-existing on master itself, or an
already-acknowledged gap being tracked separately.

Pre-existing failures (not caused by this branch)

Both of these crash identically on a plain master build, before the
feature-branch side of the comparison is even run:

  • axisymmetric: FATAL from PE 14/15: interpolator_2D : You must call interpolator_init before calling interpolator
  • giant_planet: FATAL from PE 0: spectral_dynamics: temperatures out of valid range

Neither is touched by anything in this branch's diff. Flagging for
visibility rather than fixing here, since they're out of scope for this PR.

Known, acknowledged failure: radiative_eq_mars

Fails identically on both master and this branch with an "unknown namelist
variable" error. radiative_eq_mars_test_case.py sets five namelist keys
(do_newtonian_cooling_as_rad, newt_relax_surface, update_sst_from_fluxes,
rh_target, delta_t_relax) that don't exist as declared Fortran namelist
variables anywhere in current master. These likely belong to
top_down_with_moisture (last touched 2020-04-24, ~979 commits behind
master) — the branch this test case's physics originates from — and were
lost when this test case was merged without that branch ever being
reconciled.

Plan: bring a modernized top_down_with_moisture up to date against
master on its own branch/PR, then use that to properly restore
radiative_eq_mars, rather than papering over the gap with local namelist
deletions. Until then this failure is expected.

@sit23

sit23 commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Given that all the trip tests are passing, bar the known fails, this is now ready to be merged. This means that all the Isca-Mars funcionality is now available within the main version of Isca.

@sit23
sit23 marked this pull request as ready for review August 19, 2026 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants